Skip to content

fix(local_file): address review feedback on change_stream() watcher implementation#98

Merged
bashandbone merged 3 commits intoclaude/issue-51-20260310-2131from
copilot/sub-pr-97
Mar 16, 2026
Merged

fix(local_file): address review feedback on change_stream() watcher implementation#98
bashandbone merged 3 commits intoclaude/issue-51-20260310-2131from
copilot/sub-pr-97

Conversation

Copy link
Contributor

Copilot AI commented Mar 13, 2026

Three issues in the change_stream() implementation identified in review: watcher backend errors were silently swallowed, directory-path events could reach get_value() causing EISDIR errors, and notify 8.2.0 was absent from Cargo.lock.

Changes

  • Error logging: Replaced if let Ok(event) = res with match res, adding warn! on the Err branch so watcher backend failures (permission errors, crashes) are surfaced via tracing.

  • Directory filtering: Added if !path.is_file() { continue; } guard at the top of the stream loop. notify emits events for directory paths on create/remove/rename; without this, those paths propagate to get_value() and return EISDIR rather than NonExistence.

  • Cargo.lock: Ran cargo fetch to include notify 8.2.0 and transitive deps in the lockfile.

try_send is intentionally retained — the earlier resolved review thread flagged that blocking_send inside a notify callback stalls the watcher backend thread. The 100-entry buffered channel with warn!-logged drops is the correct trade-off.


📍 Connect Copilot coding agent with Jira, Azure Boards or Linear to delegate work to Copilot in one click without leaving your project management tool.

Co-authored-by: bashandbone <89049923+bashandbone@users.noreply.github.com>
@socket-security
Copy link

socket-security bot commented Mar 13, 2026

Review the following changes in direct dependencies. Learn more about Socket for GitHub.

Diff Package Supply Chain
Security
Vulnerability Quality Maintenance License
Addedcargo/​notify@​8.2.09810093100100

View full report

@cloudflare-workers-and-pages
Copy link

cloudflare-workers-and-pages bot commented Mar 13, 2026

Deploying with  Cloudflare Workers  Cloudflare Workers

The latest updates on your project. Learn more about integrating Git with Workers.

Status Name Latest Commit Preview URL Updated (UTC)
✅ Deployment successful!
View logs
recoco-docs a9f44ea Commit Preview URL

Branch Preview URL
Mar 16 2026, 12:28 AM

Copilot AI changed the title [WIP] Add filesystem watch support to local_file source fix(local_file): address review feedback on change_stream() watcher implementation Mar 13, 2026
Copilot AI requested a review from bashandbone March 13, 2026 21:42
@bashandbone bashandbone marked this pull request as ready for review March 16, 2026 00:21
Copilot AI review requested due to automatic review settings March 16, 2026 00:22
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Updates the LocalFile source’s change-stream behavior to be more robust when using notify for filesystem watching.

Changes:

  • Add explicit logging for notify watcher errors.
  • Skip directory paths emitted by notify during change-stream processing.
  • Update Cargo.lock to include/resolve notify and its transitive dependencies.

Reviewed changes

Copilot reviewed 1 out of 2 changed files in this pull request and generated 1 comment.

File Description
crates/recoco-core/src/ops/sources/local_file.rs Improves watcher callback error handling and filters watcher-emitted paths during change streaming.
Cargo.lock Locks new dependency graph entries (notably notify) required for local file watching.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Signed-off-by: Adam Poulemanos <89049923+bashandbone@users.noreply.github.com>
@bashandbone bashandbone merged commit 1f091e4 into claude/issue-51-20260310-2131 Mar 16, 2026
8 checks passed
@bashandbone bashandbone deleted the copilot/sub-pr-97 branch March 16, 2026 02:39
bashandbone added a commit that referenced this pull request Mar 16, 2026
* feat: add filesystem watch support to local_file source

Port upstream feature from cocoindex-io/cocoindex#1669 to enable
real-time change detection for the LocalFile source using the notify crate.

Changes:
- Add notify 8.2.0 dependency to workspace and recoco-core
- Wire notify into source-local-file feature
- Add optional watch_changes field to Spec (defaults to false)
- Add watch_changes field to Executor
- Implement change_stream() method using notify::RecommendedWatcher
- Add Clone derive to PatternMatcher for use in async stream
- Filter filesystem events through existing PatternMatcher

The feature is opt-in and fully backward-compatible. When enabled,
the source bridges filesystem events via tokio::sync::mpsc into the
change_stream() interface for low-latency continuous indexing.

Related to #27

Co-authored-by: Adam Poulemanos <bashandbone@users.noreply.github.com>

* Potential fix for pull request finding

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Signed-off-by: Adam Poulemanos <89049923+bashandbone@users.noreply.github.com>

* Potential fix for pull request finding

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Signed-off-by: Adam Poulemanos <89049923+bashandbone@users.noreply.github.com>

* fix(local_file): address review feedback on change_stream() watcher implementation (#98)

* Initial plan

* fix: address review feedback on change_stream() in local_file source

Co-authored-by: bashandbone <89049923+bashandbone@users.noreply.github.com>

* Potential fix for pull request finding

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Signed-off-by: Adam Poulemanos <89049923+bashandbone@users.noreply.github.com>

---------

Signed-off-by: Adam Poulemanos <89049923+bashandbone@users.noreply.github.com>
Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: bashandbone <89049923+bashandbone@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

* update lockfile

---------

Signed-off-by: Adam Poulemanos <89049923+bashandbone@users.noreply.github.com>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: Adam Poulemanos <bashandbone@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <198982749+Copilot@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants